Skip to content

fix(rest-api): allow tenants to use provider templated OSes - #4489

Open
pbreton wants to merge 2 commits into
NVIDIA:mainfrom
pbreton:codex/4486-provider-os-tenants
Open

fix(rest-api): allow tenants to use provider templated OSes#4489
pbreton wants to merge 2 commits into
NVIDIA:mainfrom
pbreton:codex/4486-provider-os-tenants

Conversation

@pbreton

@pbreton pbreton commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What changed

  • allow tenants to select provider-owned templated iPXE Operating Systems when creating, updating, or batch-creating Instances
  • keep tenant-owned Operating Systems private to their owning tenant
  • retain the existing requirement that a provider-owned templated OS has a Synced association with the Instance site
  • add regression coverage for all three Instance paths

Root cause

Provider-managed templated Operating Systems intentionally have no tenant_id. The Instance OS ownership checks called String() on that nil value and required every selected OS to be tenant-owned, even though the Operating System list API intentionally exposes provider-managed definitions to tenants at associated sites.

Impact

Tenants can now use provider-managed templated iPXE definitions that are synchronized to their Instance site. Cross-tenant definitions and provider definitions unavailable at that site remain rejected.

Validation

  • reproduced the failure with a regression test; the create path panicked on the nil provider OS tenant_id
  • go test ./api/pkg/api/handler -run '^TestBuildInstanceOsConfig_TemplatedIPXE$' -count=1
  • go test ./api/pkg/api/handler -count=1
  • make test from rest-api/
  • git diff --check

Fixes #4486

@copy-pr-bot

copy-pr-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e3d369c1-b0a5-4e2d-affb-cd7db0dc3f3c

📥 Commits

Reviewing files that changed from the base of the PR and between ce2ebf5 and 00f841c.

📒 Files selected for processing (4)
  • rest-api/api/pkg/api/handler/instance.go
  • rest-api/api/pkg/api/handler/instancebatch.go
  • rest-api/db/pkg/db/model/operatingsystem.go
  • rest-api/db/pkg/db/model/operatingsystem_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • rest-api/api/pkg/api/handler/instancebatch.go
  • rest-api/api/pkg/api/handler/instance.go

Summary by CodeRabbit

  • New Features

    • Provider-owned, synchronized Templated iPXE operating systems can now be used when creating or updating instances.
    • Batch instance creation supports these operating systems and preserves their operating system configuration.

  • Bug Fixes

    • Improved operating system authorization while retaining site-association validation for applicable configurations.
    • Prevented unauthorized use of provider-owned operating systems that do not meet the required configuration criteria.

Walkthrough

The change adds OperatingSystem.IsTenantUsable and applies it to instance creation, updates, and batch creation. Eligible provider-owned Templated iPXE Operating Systems now pass validation. Tests cover ownership rules and instance configuration paths.

Changes

Operating System authorization

Layer / File(s) Summary
Operating System usability rule
rest-api/db/pkg/db/model/operatingsystem.go, rest-api/db/pkg/db/model/operatingsystem_test.go		 IsTenantUsable accepts matching tenant-owned records and provider-owned Templated iPXE records with an infrastructure provider. Tests cover accepted and rejected ownership and type combinations.
Instance handler integration
rest-api/api/pkg/api/handler/instance.go, rest-api/api/pkg/api/handler/instancebatch.go, rest-api/api/pkg/api/handler/instance_test.go		 Create, update, and batch-create validation use Operating System usability. Tests verify that synchronized provider-owned Templated iPXE records produce ID-based configurations.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the change to allow tenants to use provider-owned templated Operating Systems.
Description check ✅ Passed The description accurately explains the fix, constraints, affected paths, regression coverage, and validation.
Linked Issues check ✅ Passed The changes satisfy issue #4486 by allowing tenants to use provider-created templated Operating Systems while preserving access restrictions.
Out of Scope Changes check ✅ Passed All code and test changes directly support provider-owned templated Operating System usability and the linked issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@pbreton
pbreton marked this pull request as ready for review August 3, 2026 17:14
@pbreton
pbreton requested a review from a team as a code owner August 3, 2026 17:14
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

✅ No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-08-03 17:17:05 UTC | Commit: 28975e4

@thossain-nv thossain-nv added the rest-api Add this label when an issue or PR concerns NICo REST API label Aug 3, 2026 — with ChatGPT Codex Connector
thossain-nv
thossain-nv approved these changes Aug 4, 2026
View reviewed changes

@thossain-nv thossain-nv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix @pbreton, one suggestion.

Comment thread rest-api/api/pkg/api/handler/instance.go Outdated
@pbreton
fix(rest-api): allow tenants to use provider templated OSes
ce2ebf5 
Signed-off-by: Patrice Breton <pbreton@nvidia.com>
@pbreton
pbreton force-pushed the codex/4486-provider-os-tenants branch from 28975e4 to ce2ebf5 Compare August 4, 2026 22:14
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🌿 Preview your docs: https://nvidia-preview-pull-request-4489.docs.buildwithfern.com/infra-controller

@pbreton
refactor(rest-api): move OS usability to model
00f841c 
Signed-off-by: Patrice Breton <pbreton@nvidia.com>
@pbreton
pbreton enabled auto-merge (squash) August 5, 2026 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spydaNVIDIA spydaNVIDIA spydaNVIDIA approved these changes
@thossain-nv thossain-nv thossain-nv approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

rest-api Add this label when an issue or PR concerns NICo REST API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: provider-created Operating Systems are not usable by tenants

3 participants

@pbreton @spydaNVIDIA @thossain-nv